Skip to content

fix(plugin-security)!: evaluate the insert-side RLS check on the row that will be stored, after beforeInsert - #16805

Merged
os-bill merged 14 commits into
mainfrom
claude/issue-16608-insert-check-post-image
Sep 9, 2026
Merged

fix(plugin-security)!: evaluate the insert-side RLS check on the row that will be stored, after beforeInsert#16805
os-bill merged 14 commits into
mainfrom
claude/issue-16608-insert-check-post-image

Conversation

@os-trump

@os-trump os-trump commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16608

The insert-side RLS check is evaluated on the row that will be stored — after beforeInsert — instead of on the caller's raw payload, so insert and update judge the same thing.

Clause-②: yes — the security middleware's accept / refuse behaviour changes. Stays in draft: maintainer-only merge.

Contract review: DONE, and its blocking finding is fixed in this branch. An isolated CONTRACT_REVIEW_TIER seat reviewed head cd09d3b99 and returned CHANGES REQUIRED on one finding (comment 5581120293). That finding is F1 below; the patch round that answers it is the second half of this body. needs:contract-review was removed from both carriers when the review concluded — re-hanging it against the patched head is the PM seat's step, not this branch's.


The ruling this implements

Maintainer, 2026-09-07, director seat, summon #17, decision batch #3, verbatim: 「同意」 (issue comment 5572345610).

The security middleware's insert post-image becomes the hook-mutated payload — the row that will be stored — so insert and update judge the same thing.

⛔ Option 2 (keep the order and write the contract that a checked field must arrive from the caller, plus an os validate rule) is refused, not deferred: it institutionalises the contradiction — the caller sending the value the hook exists to make un-sendable — and adds a permanent rule to keep it. Nothing here drifts back to it.

The two measurements the ruling required first

Both were taken on origin/main @ 941232040 before the evaluation point moved, and posted in full on the card (comment 5579761278). Summary:

① The bypass case — IT REPRODUCES. The ruling flagged this as reasoning, not measured. It is now measured, on a real engine with a real SecurityPlugin and a real driver. A caller holding org_a sends employer_org: org_a on a child whose parent belongs to org_b; the app's runAs: 'system' stamp reads the parent outside RLS and overwrites the field.

payload before this PR stored row (read off the driver's table)
{ employer: 'emp_b', employer_org: 'org_a' } ADMITTED { employer: 'emp_b', employer_org: 'org_b' }
{ employer: 'emp_a' } — left to the hook (the card's row 2) 403 PERMISSION_DENIED nothing stored
{ employer: 'emp_a', employer_org: 'org_a' } (the card's row 1) ADMITTED employer_org: 'org_a'

Today's order admits a row whose stored scope the caller does not hold. That is a cross-organization write, and it is what regrades the card to priority:p1. After this PR: row 1 is refused with nothing stored, row 2 is admitted and stores org_a, row 3 is unchanged.

② The census — non-zero, and large: 29 hooks. Reported as a fork, with the list, on the card. Every beforeInsert hook whose derivation reads a caller-supplied field: 4 in this repo, 7 in ats @ c3c6526, 18 in hotcrm @ d47e37a. The per-hook table is on the card.

⛔ Not a silent fallback to option 2. What this PR installs instead of 29 per-hook declarations is one invariant at the gate, which is exactly the guard the census was asking for. Its precise statement is in the changeset, and it is narrower than the first draft claimed — see F1.


What changed

@objectstack/objectqlOperationContext.postHookWriteImageCheck. An optional judgement an enforcement layer installs and ObjectQL.insert runs once the beforeInsert chain has produced the row. It sits after the post-hook declared-field door (#13657), after the two value-changing strips, and before every producer with a side effectresolveSystemInsertOrganization, encryptSecretFields (which writes a sys_secret row), applyAutonumbers (which CONSUMES a sequence number), validation, the statement — so a refusal still costs nothing, the same rule #8682 wrote for the door. Rows the declared-field door culled from a partial batch are skipped: they will not be written. honoured is set before evaluate, so a throwing check still reads as honoured — the flag answers "did the seam run", never "did the write pass".

@objectstack/plugin-security — step 3.6 installs instead of matching. For insert the compiled check filter goes on the operation context; for update nothing changes (that path already merges its caller pre-image, and the ADR-0090 D10 delegator half rides along unchanged). The filter is still compiled in the middleware, where the caller's permission sets, the delegator's, the staged membership and this request's context are all resolved. Only the IMAGE is deferred; deferring the compilation would move authorization inputs into the engine's timeline for no gain. Both verbs share one refusal closure, so a caller cannot tell which side judged.

Fail-closed on a seam that never runs. A middleware that installed the judgement and finds honoured unset refuses the write and logs at ERROR, with a developer message that says the check was not evaluated rather than that it failed — different facts, and an operator debugging one must not be handed the other. ⛔ Deliberately not softened to a warning: a middleware that cannot say a write was checked must not report that it was.

Scope kept, not widened. Only objects governed by a permission set that EXPLICITLY declares check, single-row inserts, non-system caller — the gate's existing scope. Batch inserts were never post-image validated here and still are not (now filed as #16877).


The contract-review patch round

Review: PR comment 5581120293 (isolated CONTRACT_REVIEW_TIER seat, head cd09d3b99). Handoff: card comment 5582300461.

F1 (BLOCKING) — the row the seam judged was not the row that was stored

What the review measured. The seam was placed correctly with respect to side effects but not with respect to value changes. Two engine passes still ran after it, and both can change a value a caller sent:

A check over a readonly scoping field — which is the natural shape, because ADR-0055 forces the predicate onto the denormalised column and readonly: true is how an author says "not yours to send" — therefore judged the caller's in-scope value while the store received the field's defaultValue, or NULL if it had none.

Route taken: the review's PREFERRED fix, not the alternative. Both strips move ahead of the seam. They are side-effect-free — they read suppliedPerRow, rowHookWrittenKeys, the schema and options, all resolved above the seam, and they log — so the move costs nothing and buys the seam the final row.

⚠️ One deliberate deviation from a naive reading of "move the strips": the reporting half stays where it was. insertDropped is still discharged after the seam (strictReadonlyWrites refusal, onFieldsDropped), because moving it too would put ReadonlyFieldRejectedError ahead of the gate's 403 and change which refusal a caller sees. The strips are value-changing; their report is not.

What the reorder actually moves — measured on BOTH legs, not reasoned. Three engine passes now see the stripped row where they used to see the unstripped one. The two that are observable were measured by checking cd09d3b99's engine.ts out over this tree and running the same cells against it (on-disk swap proven by blob hash, restored under a trap, git diff HEAD empty afterwards):

  1. resolveSystemInsertOrganization — a caller-forged value on a readonly tenant column no longer suppresses the platform's organization derivation. A narrowing; read off the code path, not separately exercised.

  2. encryptSecretFields — this one found a pre-existing hole, and the reorder closes it. A caller-forged value on an author-declared readonly secret field was encrypted and stored on the reviewed order:

    cd09d3b99 (reviewed order) after the reorder
    stored token "secret:sec_1" null
    ICryptoProvider.encrypt calls 1 0
    sys_secret rows minted 1 0

    The mechanism: the credential channel ran first and replaced the row's value with a reference (row[field] = makeSecretRef(handle.id)), so the strip's Object.is value test then compared that reference against the caller's plaintext, read the difference as "a hook rewrote this key", and kept the forgery. That is the one input on which that test inverts. It predates this card — it is 17.3.0's behaviour — and it is now a cell.

  3. ⚠️ refuseEmptyPasswordFields — the one direction that is NOT a narrowing. A readonly password field carrying '' answered VALIDATION_ERROR ("Empty string refused for password field") on the reviewed order and is stripped here, with the row admitted and pw stored as NULL. '' reaches the store on neither order, so the 2026-08-13 empty-credential ruling's guarantee — a masked column must never read as "set" while holding nothing — is untouched; what moves is which refusal a caller sees, on a payload the caller was never allowed to send. ⛔ Deliberately not "fixed" by moving refuseEmptyPasswordFields up as well: that would let a field-level validation verdict answer a write the RLS gate refuses, which is the wrong precedence for a security gate. Flagged for the maintainer rather than buried; it is a cell, and the changeset carries it.

The invariant is now stated to its real edge, and the claim is not deleted. The changeset said "a stored row always satisfies the insert check, whatever the caller sent". The review's verdict on that sentence was that it is false. It now reads: a stored row satisfies the insert check on every field the caller can steer — and it names the four engine-owned passes that still substitute a platform value afterwards (the tenant fill of an absent column, the sys_secret reference, the autonumber, multi-value normalisation), so a policy whose check names one of those fields is documented as a boundary rather than covered by a promise. engine.ts's seam comment carries the same closed list; the sentence "Nothing between here and the driver adds a value the caller could have steered" is gone, because it was the sentence that was false.

F4 — the two claims that had no pin

Both are new cells, on the same both-drivers footing as the rest of the file.

  • (i) an unevaluable check refuses. A policy whose check names a current_user.* key no resolver publishes compiles to RLS_DENY_FILTER, the fail-closed sentinel that matches no row. The cell asserts the refusal on the ADR-0112 envelope with nothing stored — and asserts it through the seam: the beforeInsert stamp is observed to have run, which it could only have done if the middleware handed the write to the engine, so the refusal is evaluate's and not the middleware's.
  • (ii) a refusal costs nothing. An object carrying both an autonumber and a secret field, with a real reversible ICryptoProvider wired so encryptSecretFields runs its real path. After a refused insert: nothing stored, encrypt call count 0, sys_secret table empty. Then the same insert admitted: the survivor's record number equals the number a control boot that never refuses anything hands its first row — so the refused attempt drew no sequence value — and exactly one encrypt call with exactly one sys_secret row, which is also the positive control that the field really is on the credential path.

F2 / F3 — filed, not fixed here

F6 — the lockfile carries more than the one devDependency add

Noted rather than dropped, which is the review's first option. Beyond the @objectstack/driver-sqlite-wasm devDependency this PR adds, pnpm-lock.yaml collapses 7 esbuild@0.28.1 peer-resolution entries (11 references at the merge base, 4 now). It is unrelated normalisation that a plain pnpm install produced; Install / Build / Validate Package Dependencies are green on it. ⛔ Not hand-edited back: a lockfile edited to look tidier than the resolver's own answer is a worse artifact than an untidy honest one.


The conformance case

packages/plugins/plugin-security/src/insert-check-post-image.test.ts25 cells, all green (13 from the first delivery, 12 added by this patch round).

The original proposition, the scoping field's landing decides, is written once and asserted for both verbs on both driver families (driver-sql better-sqlite3 :memory:, and driver-sqlite-wasm):

  • lands IN scope, admitted, and the stored row carries that value;
  • lands OUT of scope, refused on the ADR-0112 envelope (PERMISSION_DENIED / 403 / the catalog sentence / the developer line), and nothing moved — refusal and non-landing are asserted as separate facts, because a gate that refuses after the row lands is not a gate.

The insert arm's out-of-scope cell differs from its in-scope twin by the parent alone: both payloads carry an in-scope employer_org, so they are indistinguishable to the pre-hook image and no green there can come from it. Plus the card's two rows (bare payload admitted; duplicated stamp still admitted), and the fail-closed leg against an engine double that ignores the seam.

The twelve new cells, each on both drivers:

  • F1 — the readonly-scoping-field pair: no defaultValue, and a defaultValue naming an organization the caller does not hold. Each asserts the review's disjunction over the driver's own table (either the insert is refused or the stored row satisfies the check) before pinning the answer the runtime gives, so the cell states the invariant rather than a verdict.
  • F4 (i) — the unevaluable-check cell.
  • F4 (ii) — the cost cell.
  • the reorder's own consequences — the readonly secret forgery and the readonly password empty string, both with their reviewed-order readings quoted in the cell so the change of behaviour is legible from the test rather than only from this body.

Ablation

Two ablations, one per delivery. Both mutate on disk, prove the mutation reached the disk before reading any result, and restore under a trap ... EXIT INT TERM. Both packages resolve to SOURCE in this package's vitest config (@objectstack/objectql is aliased there), so no build stands between the edit and the run.

Round 1 — the seam itself. Mutating the installed judgement to read opCtx.data instead of the rows the engine hands it:

predicted: RED on exactly the four insert cells whose verdict differs between the two images
measured:  Tests  4 failed | 9 passed (13)

Round 2a — the F1 fix, minimally reverted. The seam judges a snapshot of the rows taken before the strips, which is the reviewed head's behaviour with everything else unchanged.

HEAD blob      : ac8fed120c44b3dbe0fc3a8343d263792bf37de9
on disk before : ac8fed120c44b3dbe0fc3a8343d263792bf37de9   (equal — the tree was at HEAD)
anchor `__ablationPreStrip`                      : 0 -> 2
anchor `live.push(rowHookContexts[i]!.input...)` : 1 -> 0
on disk after  : 5008ec636515a30fd81a8d5fd0556adeb0a4f996   (differs — ON-DISK MUTATION PROVEN)

predicted: RED on exactly the four F1 cells (2 cells x 2 drivers), green everywhere else
measured:  Tests  4 failed | 17 passed (21)
           x no default (driver-sql)          x foreign default (driver-sql)
           x no default (sqlite-wasm)         x foreign default (sqlite-wasm)

  AssertionError: the strip's re-default must not be able to store an organization
  the caller does not hold: expected true to be false

restore: git checkout HEAD -- ABSPATH
on disk after restore : ac8fed120c44b3dbe0fc3a8343d263792bf37de9   (byte-identical to the HEAD blob)
git diff HEAD         : empty

⭐ Read the failure text: with the fix reverted, the row lands, carrying org_b, for a caller holding only org_a. F1 is this card's own headline defect one layer down, and that is the cell that measures it.

Round 2b — the whole conformance file against the reviewed head's engine. cd09d3b99's engine.ts checked out over this tree, which is the strongest available "before":

HEAD blob (fixed order)      : ac8fed120c44b3dbe0fc3a8343d263792bf37de9
cd09d3b99 blob (review head) : 196e87e52b24a1aa943417d2297ae1817d2569a6
on disk after swap           : 196e87e52b24a1aa943417d2297ae1817d2569a6   (differs from HEAD — swap proven)

measured: Tests  6 failed | 17 passed (23)   [before the two consequence cells were widened to both drivers]
          the 4 F1 cells, plus both reorder-consequence cells

restore: git checkout HEAD -- ABSPATH
on disk after restore : ac8fed120c44b3dbe0fc3a8343d263792bf37de9   (byte-identical)
git diff HEAD         : empty

Every cell this patch round adds is therefore MEASURED: each one fails on the head the review read, and passes here.

Tests and gates

Measured on the pushed head e6dad93b5, after merging origin/main (a merge commit, never a rebase — this branch is published).

pnpm --filter @objectstack/plugin-security test 103 files / 1927 tests passed
pnpm --filter @objectstack/objectql test 286 files / 4815 tests passed
typecheck, both packages green; both test layers compile (plugin-security at 0 errors / 0 ledger entries)
dispatch-gates.mjs --ran reconciliation 83 derived, 83 run, 0 NOT-MEASURED, 0 UNRUN

dispatch-gates reported STALE TREE repeatedly (first 52 commits behind with 19 gate-source files changed, then 5 more), so origin/main was merged in — a merge commit, never a rebase — and the union re-derived from the fresh tree each time before it was run.

⭐ Zero NOT MEASURED this round. Three gates answer exit 3 — PREREQUISITE NOT MET, neither green nor red — until the whole workspace is built: check:dual-build-cjs-loads, check:i18n and check:type-check-debt. Rather than declare them to CI, the closure was built (turbo run build --filter='./packages/*' --filter='./packages/*/*' — 72 tasks, all successful) and all three re-run to a real verdict: exit 0, exit 0, exit 0. check:type-check-debt re-measured 5 ledger entries, 55 raw tsc errors, none above its recorded number, surplus: none.

check:route-envelope is in dispatch-gates' "silent / weakest verdict" bucket (#16828) so it never appears in a derived union. Checked explicitly rather than skipped: no file this diff touches contains c.json( or res.json(, so it does not apply here.

⚠️ The residual, stated rather than papered over. origin/main advances faster than a union can be re-derived and re-run against it, so the last derivation ran one commit behind with one gate-source file changed in that range — scripts/engine-double-contract.pinned.json, which this diff also edits. origin/main was merged once more afterwards (no conflict, including in that ledger) and the families that file and the edited test file derive were re-run on the merged tree: check:engine-double-contract, check:test-source-alias, check:type-source-resolution, check:where-matcher, check:objectql-double-limit, check:query-options-erasure, check:type-check-coverage, check:swallow-census-controls, check:nul-bytes, check:doc-authoring, check:i18n-stale-fill, check:cross-package-test-inputsall exit 0. Closing the last commit of the race is CI's job, on the merge commit it builds.

pnpm lint — a proven narrowing, not a skipped run. Run over the six lintable source files this diff touches: --format json reports 6 files linted, 0 errors, 0 warnings. Independent control in this tree: parserOptions.project appears nowhere in eslint.config.mjs, and a grep for the typed @typescript-eslint rule family (no-floating-promises, no-unsafe-*, await-thenable, no-misused-promises, require-await, restrict-*) returns 0. The narrowing is sound because this repo runs one eslint.config.mjs which never enables type-aware linting for any file (no parserOptions.project, no typed @typescript-eslint rules — stated and measured with a positive control in that file's own comment at the QUERY_OPTIONS_TEST_GLOBS block), so nothing in this diff can move the verdict on a file it does not touch. The repo-wide sweep remains CI's.

Reviewer's fast path

Three hunks. In packages/objectql/src/engine.ts, insert() runs its two value-changing strips, then calls one new optional callback, then proceeds to the producers; the insertDropped reporting block is unmoved. In packages/plugins/plugin-security/src/security-plugin.ts step 3.6, the insert branch installs that callback where it used to match a filter against opCtx.data; the update branch is the old code, unmoved. Everything else is the conformance file, the doubles learning to model the engine, and gate bookkeeping.

验收备注

  • The update path has the identical hole, and this PR does not fix it. Measured on this branch, both drivers: an update that repoints the parent so the beforeUpdate stamp rewrites the checked field after the middleware merged its pre-image is ADMITTED, and the row is stored in an organization the caller does not hold. Filed as plugin-security: the UPDATE-side RLS check post-image is pre-image + change set evaluated before beforeUpdate, so a hook-stamped scoping field can move a row into an organization the caller does not hold #16790. Not fixed here on purpose: the ruling names the insert side and treats the update path as the correct reference, so extending the evaluation point to a second verb is a second accept/refuse change that wants its own decision. The mechanism this PR adds is directly reusable for it. The conformance file states out loud that it does not claim that route, rather than pinning today's answer there — a test asserting the defective behaviour would advertise a guarantee the runtime does not deliver.
  • check-clause2-carriers --pair 16805, re-read for the review's F7 — the exit-4 note this body used to carry is RETIRED, and the reason it described is FIXED. That note said the pair was clause-② illegible because the claim comment opened ## Claim: … while CLAIM_COMMENT_MARKER only accepts a claim at column 0. The PM posted the compliant spelling on 5580367981 (Claim: … · Clause-②: yes, column 0, explicitly a format correction and not a second claim), so the carrier that note sent a reader to fix no longer needs fixing. ⚠️ The live re-run is NOT MEASURED from this seat, and for a transport reason rather than a board condition: --pair 16805 exits 3 — PREREQUISITE NOT MET, GET /repos/objectstack-ai/objectstack/pulls -> HTTP 403, 0 pair(s) had been read. The family's own classifier (check-half-states.mjs --probe, which the gate names as the one thing that classifies its transport) agrees and names the fix: "run this from a container whose egress allows repo-scoped reads (CI, or the Routine seat class); in a proxy-mediated seat, repo-scoped reads stay on the mcp__github__* tools". ⛔ Not reported as a pass. What IS readable from here, over MCP: neither carrier holds needs:contract-review — both were stripped when the review concluded — and re-hanging them against the patched head is the PM seat's step per the handoff.
  • noted, not filed: predicate (multi: true) updates are still not post-image validated. Pre-existing, documented in the gate, unmoved here, and explicitly out of plugin-security: array inserts are never check-gated — the write gate's own guard skips any array payload, and the REST import runner reaches engine.insert with one #16877's scope — an update selected by a predicate cannot form a post-image in the middleware at all, so it is not made cheaper by this seam.
  • noted, not filed: this change gives engine doubles a new obligation — an executor that stands in for ObjectQL.insert must run the installed judgement or the middleware refuses. Three call sites in two existing test files were updated. check:engine-double-contract covers delete/update/findOne but not this seam, so a future double will learn about it from a red test rather than from that gate.

Authored by Claude Code in session session_012zTkyNHJ7TkuN2oXtP5x37.

🤖 Generated with Claude Code

https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

…ill be stored

The security middleware runs before the engine's operation, so for an insert
its post-image was the caller's payload as it arrived — ahead of every
`beforeInsert` hook. A denormalised scoping field is what an RLS predicate
compares (ADR-0055) and what an app stamps server-side so a caller cannot
choose it, so the gate judged a value that never lands and ignored the one
that does. Measured both ways on 17.3.0: a payload leaving the field to the
hook was refused while the identical payload carrying it was admitted, and an
insert naming an in-scope organization on a parent in another organization was
admitted with the parent's organization stored on it.

`OperationContext` gains `postHookWriteImageCheck`, a judgement an enforcement
layer installs and `insert()` runs once the hook chain has produced the row —
after the post-hook declared-field door, before every producer with a side
effect. plugin-security installs its compiled check filter there; the update
path, which already merges its pre-image, is unchanged. A seam that was
installed and never run refuses the write rather than vouching for it.

One conformance cell, both verbs, both drivers: the scoping field's landing
decides. Refs #16608, ruling 2026-09-07.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
The insert-side RLS check is installed on the operation context and run by
`ObjectQL.insert`; a double whose executor is a bare `async () => {}` models
an engine that carries a write past a gate that never ran, which the
middleware refuses fail-closed. The doubles in `security-plugin.test.ts` and
`rls-check-membership-staging.test.ts` now run the judgement the way the
engine does — flag first, then evaluate — so they model the engine instead of
a looser approximation of it.

Also fixes the fail-closed log call to the `error(message, error?, meta?)`
contract arg order (#5637).

Refs #16608.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
- the changeset carries its ADR-0087 disposition: an enforcement-ORDER change
  moves no authorable key, spelling or stored shape, so no conversion entry
  and nothing for an upgrader to hand-edit (check:adr-0087-registration);
- the fail-closed leg's engine double routes delete/update/findOne through the
  real dispatch predicates, so it cannot be looser than ObjectQL
  (check:engine-double-contract);
- `@objectstack/driver-sqlite-wasm` — the conformance cell's second driver
  family — is read from the producer's SOURCE on both axes: a vitest alias
  (check:test-source-alias) and a bare-key tsconfig `paths` rule
  (check:type-source-resolution). Measured: the paths route adds zero
  diagnostics from other packages here; the test layer still compiles at 0.

Refs #16608.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…untime string

`check:engine-double-contract --write` records the three newly-pinned seams in
the #16608 conformance file's engine double — new pinned coverage the ledger
had not learned about yet.

The fail-closed developer message no longer carries the tracker id: a runtime
string reaches authors and operators, none of whom can resolve `#NNNN`
(check:doc-authoring). The id stays in the adjacent comment, where the reader
who can resolve it is already looking.

Refs #16608.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/objectql, @objectstack/plugin-security, touching 5 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/plugins/plugin-security/tsconfig.json, packages/plugins/plugin-security/vitest.config.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/data-flow.mdx (via OperationContext (symbol, a top-level interface))
  • content/docs/permissions/system-context.mdx (via OperationContext (symbol, a top-level interface))
What this run could not see
  • 2 changed file(s) yielded no anchor (packages/plugins/plugin-security/tsconfig.json, packages/plugins/plugin-security/vitest.config.ts) — pages documenting those are invisible to this run
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 26 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 513c49556028d446db456877e0de04a92db154d9packageMentionDocs.

Which tree this was computed on

This run read content/docs from 4d20f066e0e10c0c9c83943dbf5759e4a4730838 — the merge of head 7303fe3de5156429a9e66718c63489501064b600 into base 513c49556028d446db456877e0de04a92db154d9, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 4d20f066e0e10c0c9c83943dbf5759e4a4730838 && git checkout 4d20f066e0e10c0c9c83943dbf5759e4a4730838
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 513c49556028d446db456877e0de04a92db154d9 7303fe3de5156429a9e66718c63489501064b600 && git checkout -B drift-repro 513c49556028d446db456877e0de04a92db154d9 && git merge --no-ff 7303fe3de5156429a9e66718c63489501064b600

node scripts/docs-audit/affected-docs.mjs --json 513c49556028d446db456877e0de04a92db154d9

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 513c49556028d446db456877e0de04a92db154d9 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026

Copy link
Copy Markdown
Contributor

Contract review (CONTRACT_REVIEW_TIER, isolated seat) — PR #16805 @ cd09d3b99

Verdict: CHANGES REQUIRED — one residual path where the row the seam judges is not the row that is stored (F1). Everything else the ruling asked for is present and verified; the remedy for F1 is small (a reorder or a narrowed claim plus a pin), and nothing found here reopens the refused option 2.

Ruling implemented: YES, exactly — the insert post-image is the hook-mutated row; the update path is untouched; option 2 is not present in any form. Nothing in the PR body was taken on trust: every claim below was re-read on refs/review/16805 (head cd09d3b99, merge-base b38821d1c) against origin/main (094b8fd9c).

The ruling (issue #16608, comment 5572345610, quoted verbatim)

Ruling recorded — 1: the insert-side RLS check is evaluated on the row as it will be written, after beforeInsert, the way the update path already judges its merged image — two measurements first (director seat, summon #17, decision batch #3, 2026-09-07)

Provenance (who / verbatim / where): maintainer, live PM chat with the director seat (session_01XesLUWmuhjuRwmU618AZ1M), 2026-09-07T14:5xZ, batch #3 presented as 1(1) · 2(1) · 3A · 4C · 5A with this card as item 1 recommending 1 (triage's four-facet block 5572151910, with its hard premise); reply, verbatim: 「同意」.

Ruled. The security middleware's insert post-image becomes the hook-mutated payload — the row that will be stored — so insert and update judge the same thing. Option 2 (keep the order and write the contract that a checked field must arrive from the caller, plus an os validate rule) is refused: it institutionalises the contradiction the card names, the caller sending the value the hook exists to make un-sendable, and adds a permanent rule to keep it.

Whose ruling: the comment is authored by the director seat (hotlong, MEMBER) and records a maintainer decision by verbatim quote (「同意」) to the seat's option 1 recommendation. So: a seat-recorded maintainer ruling, not a seat's own ruling. The two preconditions it set (bypass measurement, hook census) were both posted on the card before the evaluation point moved (comment 5579761278); the bypass reproduced (regrade to p1 executed in 5580367981), the census came back 29 and was reported as the fork the ruling asked for, not as a fallback.

Numbered verification

  1. Diff vs merge-base (git diff --stat b38821d1c..cd09d3b99, 11 files, +893/−83): .changeset/insert-check-post-image.md · packages/objectql/src/engine.ts · packages/plugins/plugin-security/{package.json,tsconfig.json,vitest.config.ts} · packages/plugins/plugin-security/src/{security-plugin.ts,security-plugin.test.ts,rls-check-membership-staging.test.ts,insert-check-post-image.test.ts} · pnpm-lock.yaml · scripts/engine-double-contract.pinned.json. Governed paths: NO — none of docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md, content/docs/releases/** is touched.

  2. Evaluation order, read on both refs. origin/main step 3.6 builds postImage = { ...opCtx.data } for insert and matches the compiled check there, before next() — i.e. before the engine's beforeInsert dispatch (security-plugin.ts:2773-2816 on main). Head: insert installs opCtx.postHookWriteImageCheck = { evaluate } (security-plugin.ts:2890-2903) and the engine calls it at engine.ts:10375-10384 — after triggerHooks('beforeInsert') (10257), after the seal (10275-10279) and the post-hook declared-field door (10339-10349), and before resolveSystemInsertOrganization (10408), encryptSecretFields (10456), the strips, validation, applyAutonumbers (10691) and the driver. executeWithMiddleware (3720-3741) hands the executor the same opCtx object the middleware saw, so the installed object and the honoured flag are one reference. The rows judged are rowHookContexts[i].input.data, the same objects rows is built from at 10440. Verified: the hook-mutated row is what the check sees, with no double evaluation (one evaluate per operation, over all live rows).

    • (a) No window for a hook-derived value: confirmed for every value a beforeInsert hook can write. Culled rows (partial batches) are skipped and never written.
    • (b) Tenant column: if the policy's check names the tenant column, the seam sees the hook-stamped value. But the Layer 0 write wall (security-plugin.ts:2990-3060) still judges opCtx.data pre-hook, and driver-sql's injectTenantOnInsert (12602-12614) fills the tenant column only when it is absent/empty — an explicit value written by a hook is stored as-is. So a hook that writes organization_id decides the stored tenant with no post-hook Layer 0 judgement. Pre-existing, outside the ruling's wording, and the seam this PR adds is the fix's obvious host — F2.
    • (c) Fail-closed: an unevaluable/malformed check compiles to RLS_DENY_FILTER (rls-compiler.ts:352-358, 460-462), which matchesFilterCondition cannot satisfy, so evaluate throws denyCheck(); an evaluate that throws propagates out of engine.insert (the seam sits outside the try at 10430) and the write is refused. honoured is set before evaluate so a throwing check reads as honoured. A seam that never runs is refused after next() (security-plugin.ts:3309-3327) on the same ADR-0112 envelope, logged at ERROR. Both directions closed.
    • (d) Update ordering: unchanged — the else branch at security-plugin.ts:2905-2919 is the pre-PR code (extractSingleIdgetCallerPreImage{ ...pre, ...opCtx.data }satisfiesCheck), still evaluated before next(). The beforeUpdate twin of this defect is measured and filed as plugin-security: the UPDATE-side RLS check post-image is pre-image + change set evaluated before beforeUpdate, so a hook-stamped scoping field can move a row into an organization the caller does not hold #16790; not pinned as today's answer — correct under PD chore: version packages #10.
    • (e) Bulk paths: insertManyinsert(array, { __partialRowErrors }) (10918-10920) reaches the seam, and the seam receives every live row. But the middleware's own guard !Array.isArray(opCtx.data) (2839) never installs the judgement for an array payload, so no array insert is check-gated — on main or on head. That path is caller-reachable: @objectstack/rest import-runner.ts batches CREATE rows through createManyData/insertManyData, and metadata-protocol createManyData (12295-12307) calls engine.insert(records, { context }). Pre-existing and noted in the PR body as "not filed" — F3.
    • Residual: one path where the checked row ≠ the stored row on a checked field — F1. After the seam, for a non-system caller, stripReadonlyFields (rule-validator.ts, invoked at engine.ts:10502-10520) deletes a caller-supplied value on a static-readonly field when it is not hook-written, and the engine then re-defaults the taken keys via applyFieldDefaults. A check over a readonly scoping field (the natural shape for a server-stamped field) therefore judges the caller's in-scope value and the store receives the defaultValue — or NULL if there is none — whenever the hook did not write the key (e.g. its early return when the parent key is absent). The seam doc's sentence "Nothing between here and the driver adds a value the caller could have steered" is true; the changeset's invariant "a stored row always satisfies the insert check, whatever the caller sent" is not. Not a cross-org write (the substituted value is metadata-owned), pre-existing under the old order too, but it is exactly the shape the ruling was written against, one layer down.
  3. Clause-②. Body carries Clause-②: yes; agreed — the middleware's accept/refuse set moves. The changeset states FROM/TO per case: (i) payload leaving the stamped field to the hook: FROM 403 TO admitted; (ii) payload naming an in-scope value on a parent in another org: FROM admitted (stored with the parent's org) TO refused, nothing stored; (iii) payload duplicating the stamp in-scope: unchanged; (iv) hosts/engine doubles that execute a checked insert without running the seam: FROM admitted TO refused (403, ERROR log) — present in the changeset text. Scope unchanged (explicit check, single row, non-system caller). Agrees with the body.

  4. Changeset. @objectstack/plugin-security: minor, @objectstack/objectql: minor; both are in the fixed group. node scripts/check-changeset-no-major.mjs --base origin/main --head refs/review/16805 → exit 0; check-adr-0087-registration.mjs[BREAKING+bang] not-required (no-migration-prescription) parsed with its reason. Level graded by hand against batch [WIP] Add query enhancements and advanced validation features #35 "WHICH LEVEL" (pr-automation.yml:667-682): objectql adds a new exported interface PostHookWriteImageCheck and a new OperationContext member → additive widening → minor ✓; plugin-security is an accept-set narrowing with no new public surface → during the launch window minor + BREAKING banner + ADR-0087 disposition ✓ (major refused by the guard). [finding] The changeset LEVEL axis is blind to every NESTED package: packages/*/src/** matches one segment, so 51 of 74 workspace packages (all drivers/services/adapters) can pair Clause-②: yes with patch and stay green #16713 blind spot applies: packages/plugins/plugin-security/src/** is nested, so the LEVEL axis records no package for it and its green is "did not look"; my own grade above stands in for it.

  5. Tests. insert-check-post-image.test.ts: the proposition is written once and run for both verbs on both driver families (13 cells). Pins that redden on revert: the insert out-of-scope cell differs from its in-scope twin by the parent alone (both payloads carry in-scope employer_org), so the pre-hook image cannot turn it green; the bare-payload cell pins "the caller need not send the stamped value"; the author's ablation (judge opCtx.data instead of the engine rows) reports exactly those 4 insert cells red, 9 green — consistent with what the cells assert, not independently re-run from this seat (no checkout). Negative controls: the duplicated-stamp cell (unchanged behaviour) and the update arm (unchanged code). Refusal AND non-landing are asserted separately off the driver's table. The fail-closed leg pins the unhonoured seam on the ADR-0112 envelope with the "not evaluated" developer text and an ERROR log. Engine doubles in two existing files now run the seam (runEngineWriteBody) and the three new pinned doubles are recorded. Typecheck: package.json typecheck runs tsconfig.test.json via check:test-typecheck, so the test layer compiles; driver-sqlite-wasm gets a source alias on both axes. No .skip/.only/.todo in the diff. Gaps: F4.

  6. CI on head cd09d3b99. 41 check runs: 33 success, 6 skipped, 2 failure — both are Part-of PR must not also close its card (check:partof-closing-keyword): four of the five commits carry Refs #16608 while the body says Fixes #16608; the gate is advisory (not a required context) and its own log says no author action clears it on a pushed branch. Everything else green, including Test Core 1-6, Type Check (source/workspace/consumer/debt), Lint & Repo Gates, Temporal Conformance, Governed Surface Queue Guard, Check Changeset. mergeable_state: unstable (draft; the advisory red). Branch is 19 commits behind origin/main (094b8fd9c), merge-base b38821d1c; not a conflict, but a re-merge before landing is the queue's call.

Findings

  • F1 — (blocking) the post-seam static-readonly strip + re-default can store a value the seam did not judge. engine.ts:10502-10520 runs after the seam at 10375. Expectation: either (preferred) move the two side-effect-free strips (stripRuntimeOwnedFields, stripReadonlyFields + its re-default) ahead of the seam — they read only suppliedPerRow, rowHookWrittenKeys, schemaForValidation and options, all resolved before it — so the seam judges the row after every value-changing pass and before every side-effecting one; and add one cell (readonly scoping field, caller sends in-scope value, hook does not write, no/foreign default) asserting the stored row satisfies the check or the insert is refused. If the reorder is judged riskier than it looks, the acceptable alternative is to narrow the changeset's and the seam doc's invariant to name the engine-owned passes that still run after the seam (tenant fill of an absent column with the caller's own org, readonly strip/re-default, autonumber, secret refs, audit stamps) and pin the readonly case as a documented boundary — but the current wording must not ship as written.
  • F2 — (not blocking, file it) the Layer 0 tenant write wall still judges the pre-hook image, and injectTenantOnInsert preserves an explicit tenant value, so a beforeInsert hook that writes the tenant column decides the stored tenant unjudged. Expectation: a card (sibling of plugin-security: the UPDATE-side RLS check post-image is pre-image + change set evaluated before beforeUpdate, so a hook-stamped scoping field can move a row into an organization the caller does not hold #16790) to install the Layer 0 write filter on the same seam; the seam already hands over the rows.
  • F3 — (not blocking, file it) array inserts are never check-gated (!Array.isArray(opCtx.data) at security-plugin.ts:2839), and createManyData/insertManyData reach engine.insert(array, { context }) from the REST import runner. On head the seam already receives all live rows, so lifting the insert half of that guard is now a small change. Expectation: file rather than "noted, not filed"; a caller-reachable route around a p1 gate should have a number.
  • F4 — (tests) two claims without a pin through the seam: (i) an unevaluable check (RLS_DENY_FILTER) refuses the insert via evaluate; (ii) "a refusal costs nothing" — no cell asserts that a refused insert consumed no autonumber and minted no sys_secret row. Expectation: one cell each, or cite the existing pin that covers it.
  • F5 — (process) check:partof-closing-keyword red on both runs: Refs #16608 in commits 674914d0d, 9f8402f64, 787e278b2, cd09d3b99; body Fixes #16608. Advisory, not clearable without a rewrite (forbidden). Expectation: maintainer weighs at landing; the squash message will carry Refs, the body's Fixes is what closes the card.
  • F6 — (hygiene) pnpm-lock.yaml carries more than the one devDependency add: 7 esbuild@0.28.1 peer-resolution entries collapse (11 → 4 references vs merge-base). Install/Build/Validate Package Dependencies are green, so it is consistent, but it is unrelated normalisation riding along. Expectation: note it in the body or drop it.
  • F7 — (stale body line) the check-clause2-carriers exit-4 note describes the ## Claim: heading; the PM has since posted a column-0 Claim: … · Clause-②: yes line (5580367981). Expectation: re-run --pair 16805 and update the body line so the next reader is not sent to fix a carrier that is already fixed.

Maintainer-only merge: yesfix(…)! breaking, needs:contract-review on both carriers, Clause-②: yes, p1 security narrowing, draft; and F1 is a change to the security seam itself. This seat did not approve, request changes, label, edit or push anything.


Generated by Claude Code

… pass

The contract review of this PR found one residual path where the row the
seam judges is not the row that is stored: `stripRuntimeOwnedFields` and
the static-`readonly` strip (with its `applyFieldDefaults` re-default) ran
AFTER the seam, so a `check` over a `readonly` scoping field judged the
caller's in-scope value and the store received the field's `defaultValue`
— or NULL. With a default naming another organization that is this card's
own headline defect one layer down: a stored row in a scope the caller
does not hold.

Both strips are side-effect-free, so they move ahead of the seam. The
reporting half (`insertDropped` -> `strictReadonlyWrites` /
`onFieldsDropped`) deliberately stays where it was, so the gate's 403
still precedes `ReadonlyFieldRejectedError` exactly as before.

The seam's own comment now names what still runs between it and the
driver as a closed list — the tenant fill, the secret reference, the
autonumber, the multi-value normalisation — instead of claiming nothing
does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

Copy link
Copy Markdown
Contributor

Contract review (claude-fable-5-1, isolated seat) — PR #16805 @ e6dad93b5

Verdict: PASS — re-review of the moved head. The blocking finding of the prior review (F1, comment 5581120293 @ cd09d3b99) is closed by the review's preferred route, the invariant is restated to its real edge, the two missing pins exist and can fail, and the two file cards exist. What the patch round changed was re-read in full (git diff cd09d3b99..refs/review/16805 confined to the PR's files: 4 files — .changeset/insert-check-post-image.md, packages/objectql/src/engine.ts, packages/plugins/plugin-security/src/insert-check-post-image.test.ts, scripts/engine-double-contract.pinned.json; everything else in that range is origin/main arriving through three merge commits). Nothing in the PR body was taken on trust; every line below was re-measured on refs/review/16805 (merge-base afa3a2698) against origin/main (21aabbc7b).

Ruling implemented: YES, still exactly — option 2 is present in no form; the update path is unmoved (security-plugin.ts did not change in the patch round).

The ruling (issue #16608, comment 5572345610, quoted verbatim once)

Ruled. The security middleware's insert post-image becomes the hook-mutated payload — the row that will be stored — so insert and update judge the same thing. Option 2 (keep the order and write the contract that a checked field must arrive from the caller, plus an os validate rule) is refused: it institutionalises the contradiction the card names, the caller sending the value the hook exists to make un-sendable, and adds a permanent rule to keep it.

(Director seat hotlong, MEMBER, recording the maintainer's verbatim 「同意」.) Handoff for this round: card comment 5582300461.

Prior findings

# State Evidence on e6dad93b5
F1 (blocking) closed — preferred route engine.ts insert(): triggerHooks('beforeInsert') 10257 → seal 10277-10281 → post-hook declared-field door 10338-10349 → stripRuntimeOwnedFields 10430 → stripReadonlyFields 10484 + applyFieldDefaults re-default 10515 → seam 10586-10595 → resolveSystemInsertOrganization 10619 → refuseEmptyPasswordFields/encryptSecretFields 10655-10656 → insertDropped report 10685-10703 → normalizeMultiValueFields/validateRecord 10718-10719 → applyAutonumbers 10746 → driver. The strips reassign rowHookContexts[i].input.data = stripped and the seam builds live from rowHookContexts[i].input.data, so the judged object IS the stripped object. Cells: the readonly-scoping-field pair (no default / foreign default) × both drivers, each asserting the disjunction over the driver's table before pinning the verdict.
F2 closed (filed) #16876, open, filed 2026-09-08T12:56Z: Layer 0 wall judges opCtx.data pre-hook + injectTenantOnInsert preserves an explicit value; names this seam as the fix's host. Matches the finding.
F3 closed (filed) #16877, open, filed 2026-09-08T12:56Z: !Array.isArray(opCtx.data) guard, createManyData/insertManyData from the import runner; batch-refusal semantics named as the decision. Matches.
F4 closed (i) [#16608 F4] … an unevaluable check refuses and (ii) … consumes no autonumber and mints no secret, both × 2 drivers — see §3.
F5 open, advisory (unchanged, unclearable) The same four commits (674914d0d, 9f8402f64, 787e278b2, cd09d3b99) carry Refs #16608; the eight patch-round commits carry no card trailer. See the landing note.
F6 closed (noted) Body §F6 names the esbuild@0.28.1 collapse; re-measured 11 → 4 references vs merge-base, plus the one driver-sqlite-wasm: link: add.
F7 closed (retired) Body's 验收备注 now says the exit-4 note is RETIRED and reports the live --pair 16805 re-run as NOT MEASURED (HTTP 403 transport), not as a pass. Honest. One new stale line in the other direction — N5.

Numbered verification

  1. F1 mechanics. Side-effect-free strips: read on head (packages/objectql/src/validation/rule-validator.ts:1206, :1392, :1519). Both helpers are copy-on-write over data, read supplied / hookWrittenKeys / the schema, delete a key, and call logger?.warn?.(…); neither throws — strictReadonlyWrites only changes the warning text, the throw is the ReadonlyFieldRejectedError at engine.ts:10692, which is unmoved and still after the seam. applyFieldDefaults is the same function already run pre-hook at 10172/10177, so no new code runs ahead of the seam. Claim holds. Reporting after the seam: confirmed (10685 > 10586); under strictReadonlyWrites an out-of-scope insert still answers the gate's 403 before ReadonlyFieldRejectedError, as before. Which refusal a caller sees changes in the case the PR names and in two strict-mode variants of it — N1 — and on one narrow tenant path — N2; no other case found (the post-hook door, validation, autonumber and the statement keep their relative order on both legs). Invariant text: the changeset now reads "on every field the CALLER can steer" and names the four engine-owned passes (tenant fill of an absent column, sys_secret reference, autonumber, multi-value normalisation); engine.ts 10548-10580 carries the same closed list; the sentence "Nothing between here and the driver adds a value the caller could have steered" is absent from the head file (grepped).

  2. The three reorder consequences. (a) resolveSystemInsertOrganization (engine.ts:4187) returns early when rows.every(carriesOrganization(row[tenantField])); on the new order a forged readonly tenant value is gone before that test, so derivation runs — narrowing, as stated, and confined to a non-system caller whose context carries no organization (system callers skip the strips entirely). Its edge is wider than the body's sentence — N2. (b) Verified on cd09d3b99:packages/objectql/src/engine.ts: the credential loop at 10455-10456 ran before stripReadonlyFields at 10570; encryptSecretFields (head 6794, same on old) sets row[field] = makeSecretRef(handle.id); the strip's if (!Object.is(result[name], supplied[name])) continue; then compared a ref to the plaintext and kept the key. Mechanism confirmed; the reorder closes it; the cell pins stored null, 0 encrypt calls, empty sys_secret. (c) Readonly password '': old order refuseEmptyPasswordFields (10455) threw EmptyCredentialWriteError; new order the strip removes pw (supplied, not hook-written, Object.is('', '')), re-default yields nothing for a field with no defaultValue, and refuseEmptyPasswordFields (head 6821) sees no key → row admitted, NULL stored. The 2026-08-13 guarantee — '' never at rest on a masked column — holds on both orders (read off the code, and pinned). This IS a Clause-② accept-set change at the engine level (FROM VALIDATION_ERROR TO admitted/NULL, on a readonly password field for a non-system caller), and the changeset states it with a ⚠️ as "the one direction that is not a narrowing". Maintainer's call whether stripping (readonly semantics) or refusing (credential semantics) should win for a payload the caller cannot legitimately send; the PR's reasoning for not moving refuseEmptyPasswordFields above the seam (a field-level verdict must not pre-empt the RLS 403) is sound.

  3. F4 cells can fail. (i) qa_unevaluable_member's policy check: 'record.employer_org in current_user.no_such_membership_key' against a resolver publishing only employer_org_ids; the cell asserts the ADR-0112 envelope, an empty table, and stampReads containing emp_a — the hook ran, so the write reached the engine and the refusal is evaluate's. A middleware that refused before next(), or a compiler that produced a passing filter, reddens it. (ii) qa_cost_member carries code: autonumber and token: secret with a reversible fake ICryptoProvider counting calls; a control boot's first admitted row supplies the baseline number; the subject boot's refused insert (employer: 'emp_b', in-scope payload) asserts nothing stored, encrypt === 0, sys_secret empty; the follow-up admitted insert asserts code === baseline, encrypt === 1, one sys_secret row (the positive control that the field is on the credential path). A seam placed after either producer reddens it. Both × both drivers. No .skip/.only/.todo in the file; 25 cells counted (13 + 12), matching the body.

  4. Patch-round file set. The only test file that changed is the conformance file; security-plugin.test.ts and rls-check-membership-staging.test.ts are byte-identical to cd09d3b99. scripts/engine-double-contract.pinned.json: the PR's own delta vs merge-base is the same 3 entries (insert-check-post-image.test.ts × delete/findOne/update) it had at cd09d3b99; the 12 further entries in cd09d3b99..head are all present on origin/main (seed-loader, list-user-invitations, mount-storage-routes) — they arrived by merge, not by --write. engine-double-contract.baseline.json (shrink-only) is untouched.

  5. Governed paths touched: NOgit diff afa3a2698..refs/review/16805 --name-only (11 files) hits none of docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md, content/docs/releases/**. Changeset: @objectstack/plugin-security: minor, @objectstack/objectql: minor; **BREAKING** banner present; <!-- adr-0087: not-required (no-migration-prescription) … --> present. Both gates run from the head's own script copies (identical to origin/main's) with --base origin/main --head refs/review/16805: check-changeset-no-major → exit 0 (no major bump; LEVEL axis NOT MEASURED without a PR payload, as before); check-adr-0087-registration → exit 0 ([BREAKING+bang] not-required (no-migration-prescription), reason parsed).

  6. CI on e6dad93b5: 41 check runs — 33 success, 6 skipped, 2 failure, both Part-of PR must not also close its card. Lint & Repo Gates was in_progress at first read and completed success at 14:58:52Z; Test Core 1-6, Type Check ×4, Temporal Conformance, Governed Surface Queue Guard, Check Changeset, Build Core/Docs all green. mergeable_state read blocked while that required check was still running and reads unstable now (draft + the advisory red) — so the blocked was the pending required context, not the partof gate, which its own log says is "absent from the required-context registry".

  7. Distance: 4 commits behind origin/main (21aabbc7b); none touches a PR file (spec/lint/docs/CHANGELOGs); git merge-tree --write-tree origin/main refs/review/16805 → clean, no conflicts. A re-merge is not needed for correctness; the queue's call.

Clause-②: yes

Accept-set cases and whether the changeset states FROM/TO: (i) hook-stamped field left off the payload — FROM 403 TO admitted: stated. (ii) in-scope value, parent in another org — FROM admitted (parent's org stored) TO refused, nothing stored: stated. (iii) duplicated in-scope stamp — unchanged: stated. (iv) host/double that never runs the seam — FROM admitted TO refused + ERROR log: stated. (v) readonly scoping field, caller sends an in-scope value, hook silent — FROM admitted with defaultValue/NULL stored (17.3.0 judged the caller's value) TO refused: entailed by the invariant sentence and the "both moved ahead of it" clause, not enumerated — N3. (vi) readonly secret forgery — FROM encrypted+stored TO stripped: stated. (vii) readonly password '' — FROM VALIDATION_ERROR TO admitted/NULL: stated, flagged as the non-narrowing direction. Scope unchanged (explicit check, single row, non-system caller).

New findings (none blocking)

  1. N1 — (minor, changeset) the strict-mode variants of (vi)/(vii) are not named. Under strictReadonlyWrites, the readonly-secret forgery goes FROM admitted (stored ref) TO ReadonlyFieldRejectedError, and the readonly-password '' goes FROM VALIDATION_ERROR TO ReadonlyFieldRejectedError — because insertDropped now contains the stripped key on both. Both are refusals of an un-sendable payload; the changeset states only the non-strict readings. Expectation: one sentence, or accept as implied; not a merge condition.
  2. N2 — (minor, doc) resolveSystemInsertOrganization's consequence has a sharper edge than "no longer suppresses derivation". On the reviewed order a non-system caller with no context organization who forged a readonly tenant column suppressed derivation AND then lost the value to the strip — a tenant-less row. On the new order derivation runs and, in a multi-organization posture, can throw SystemWriteOrganizationRequiredError. A narrowing that closes a worse outcome; unexercised, as the body says. Expectation: none required; a pin would close it.
  3. N3 — (minor, changeset) case (v) above as an enumerated FROM/TO row. Expectation: one bullet under "Who is affected"; the invariant sentence already entails it.
  4. N4 — (note) a refused insert now emits the strips' WARN lines (readonlyStripWarning / runtimeOwnedStripWarning) before the gate's 403; previously a refused write never reached the strips. Log-only; onFieldsDropped correctly does not fire on a refusal. Expectation: none.
  5. N5 — (stale body line) needs:contract-review IS on the PR now (labels read over MCP), while the body says twice that neither carrier holds it and re-hanging is the PM's step. Expectation: fold into the next body edit so the squash message (which must equal the body — landing note) is not stale on landing.

For the maintainer at landing

check:partof-closing-keyword RULE 2 is red on both runs (advisory): four commits carry Refs #16608, none carries a closing keyword, the body carries Fixes #16608. Per the gate's own measurement the repository squashes from commit messages, so the squash message must be replaced with the PR body by hand at the merge button, or the Refs trailers land in permanent history (they move no card). A queue merge does not do that replacement.

Maintainer-only merge: yesfix(…)!, Clause-②: yes, p1 security seam, draft, needs:contract-review on the PR; and the patch round moved two engine passes across the seam, which is a change to the seam's own contract. This seat did not approve, request changes, label, edit or push anything.


Generated by Claude Code

os-bill commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Queue-entry record — one non-required check red BY DESIGN on this head (director seat, summon #20, 2026-09-09T05:0xZ)

Landing authorized by the maintainer (batch #105 item 2, 「其他同意」, ruling on #16608 at 5595724657) on the contract-review PASS at e6dad93b5 (5587272402). Before this PR leaves draft, the one red on the head is dispositioned under the queue-entry rule's third case (SKILL.md 〈入队与落地〉, landed by #17021: 非必查红 … 第三种按设计而红,三条全立才可带红入队):

  • Gate: Part-of PR must not also close its card (.github/workflows/partof-closing-keyword-guard.ymlscripts/check-partof-closing-keyword.mjs), not in the required-context set.
  • Cause: RULE 2 of that script — four commit messages on this branch carry a card-bound trailer (674914d0d3, 9f8402f646, 787e278b25, cd09d3b99f: Refs #16608). The body itself is compliant (Fixes #16608, line 1; no Part of).
  • Condition 1 — the source says the red is by design on a pushed branch: the script's own docblock: 「⛔ Nothing in this gate's output asks anyone to rewrite history: amend, rebase and force-push are forbidden here … The red is PERMANENT for that branch」 and 「The check run is advisory at the branch-protection layer — absent from the required-context registry」.
  • Condition 2 — it does not run on merge_group: the workflow subscribes to pull_request only (types: [opened, edited, reopened, synchronize]); its header states 「No merge_group: trigger, and that is not an oversight」. The squash message assembled at merge is the queue's, not this run's.
  • Condition 3 — gate and cause recorded on the PR: this comment.

Every other check on e6dad93b5: 29 success · 4 skipped · 0 other failures (latest-per-name, read 04:1xZ). ⇒ the PR goes ready and into the queue with this one red recorded. ⛔ History is not rewritten to clear it.


Generated by Claude Code

@os-bill
os-bill marked this pull request as ready for review September 9, 2026 04:26
…sert-check-post-image

# Conflicts:
#	pnpm-lock.yaml
@os-bill
os-bill added this pull request to the merge queue Sep 9, 2026
Merged via the queue into main with commit a016f08 Sep 9, 2026
34 of 35 checks passed
@os-bill
os-bill deleted the claude/issue-16608-insert-check-post-image branch September 9, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

4 participants